Repository navigation
gh-81954: Eagerly import warnings module to failure of ZipFile.__del__ during finalization - #159419
Open
brittanyrey wants to merge 8 commits into
Open
brittanyrey wants to merge 8 commits into
brittanyrey wants to merge 8 commits into
Conversation
ZipFile.__del__ emitted its ResourceWarning before calling close(). When warnings.warn() raised, close() was skipped and the central directory of a writable archive was never written, leaving a corrupt file. This happens at interpreter shutdown, where resolving the lazy "warnings" import raises ImportError, and with -W error::ResourceWarning at any time. Call close() in a finally block.
Contributor
Author
|
Thanks for the review Sully @msullivan ! |
cmaloney
reviewed
Oct 15, 2026
cmaloney
reviewed
Oct 15, 2026
cmaloney
reviewed
Oct 15, 2026
vstinner
reviewed
Oct 15, 2026
brittanyrey
changed the title
Contributor
Author
|
Updated! |
cmaloney
previously approved these changes
Oct 15, 2026
cmaloney
left a comment
cmaloney
left a comment
Contributor
There was a problem hiding this comment.
The NEWS is a little bit long but overall looking good. I like the comment about why the import warnings is not lazy.
vstinner
reviewed
Oct 16, 2026
vstinner
left a comment
vstinner
left a comment
Member
There was a problem hiding this comment.
You reintroduced a bug when you simplified your change.
Keep a reference to warnings.warn as a default argument, like subprocess.Popen.__del__, and always call close() even if emitting the ResourceWarning raises. Verify the archive contents after collection so the test catches a skipped close(). Shorten the NEWS entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BUG: Now that
ZipFiledeliversResourceWarningsduring__del__calls, if there was an issue lazily loadingwarningsduring finalization, theclose()call was missed entirely. This left the file in an unexpected state and introduced a possibility of data loss that didn't exist previously.FIX: Eager import warnings